ROX-33217: Instrument inode tracking on directory being created path mkdir#465
Conversation
| struct inode* parent_inode_ptr = BPF_CORE_READ(parent_dentry, d_inode); | ||
| inode_key_t parent_inode = inode_to_key(parent_inode_ptr); | ||
|
|
||
| if (should_track_mkdir(parent_inode, path) != PARENT_MONITORED) { |
There was a problem hiding this comment.
I originally had
if (should_track_mkdir(parent_inode, path) == NOT_MONITORED) {
However, test_nonrecursive_wildcard::test_wildcard.py failed with that change. The reason was that was the inodes of all directories created in that test were tracked and subsequently all files in those directories were tracked regardless if they matched the wildcard pattern or not.
| #define FMODE_PWRITE ((fmode_t)(1 << 4)) | ||
| #define FMODE_CREATED ((fmode_t)(1 << 20)) | ||
|
|
||
| // File type constants from linux/stat.h |
There was a problem hiding this comment.
Could we add a permalink to the definition?
| umode_t mode = BPF_CORE_READ(inode, i_mode); | ||
| if (!S_ISDIR(mode)) { | ||
| bpf_map_delete_elem(&mkdir_context, &pid_tgid); | ||
| m->d_instantiate.ignored++; | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
We should check if m->d_instantiate.ignored actually increases, I have a feeling this condition should never be met.
|
|
||
| m->d_instantiate.total++; | ||
|
|
||
| if (inode == NULL) { |
There was a problem hiding this comment.
We should check if inode can actually be null, if so, we probably still want to try and cleanup the mkdir_context map entry.
| } | ||
|
|
||
| // d_instantiate doesn't support bpf_d_path, so we use false and rely on the stashed path from path_mkdir | ||
| __submit_event(event, m, FILE_ACTIVITY_CREATION, filename, inode, parent_inode, false); |
There was a problem hiding this comment.
Calling __submit_event will cause a full event to be filled in, including process information. However, mkdir is mostly needed for inode tracking and probably not interesting enough to send an event over gRPC, so we might want to look into having an abbreviated version that will just submit a FILE_ACTIVITY_MKDIR event that will be processed by HostScanner and dropped after adding to the userspace map.
| struct { | ||
| __uint(type, BPF_MAP_TYPE_PERCPU_ARRAY); | ||
| __type(key, __u32); | ||
| __type(value, struct mkdir_context_t); | ||
| __uint(max_entries, 1); | ||
| } mkdir_context_heap SEC(".maps"); | ||
|
|
||
| __always_inline static struct mkdir_context_t* get_mkdir_context() { | ||
| unsigned int zero = 0; | ||
| return bpf_map_lookup_elem(&mkdir_context_heap, &zero); | ||
| } |
There was a problem hiding this comment.
I think you also need to add your metrics here:
Lines 123 to 127 in eb0b3a9
I will look into improving how we handle these, it is getting very repetitive and having it scattered among multiple files is very error prone.
| os.mkdir(level1) | ||
| os.mkdir(level2) | ||
| os.mkdir(level3) |
There was a problem hiding this comment.
[nit] We can probably just use os.makedirs on level3.
| from event import Event, EventType, Process | ||
|
|
||
|
|
||
| def test_mkdir_nested(monitored_dir, server): |
There was a problem hiding this comment.
Maybe we can parametrize the final directory with UTF-8 characters?
2 participants
Description
Directory creation events need to be handled correctly. When a directory is created in a tracked directory its inode should be added to a hash set in kernel space. In user space an entry needs to be added into a map with the inode as the key and file path as the value.
An alternative approach can be found at #449
Checklist
Automated testing
If any of these don't apply, please comment below.
Testing Performed
CI is sufficient.